test(e2e): add Playwright coverage for Raise Dispute flow#298
Conversation
|
@DevSolex is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
@DevSolex Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds shared Playwright mock helpers, updates ChangesRaise Dispute E2E coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/raise-dispute.spec.ts`:
- Around line 29-35: The button-visibility coverage in raise-dispute.spec.ts
only exercises the MOCK_BOUNTY fixture with IN_PROGRESS status, so add a
parallel UNDER_REVIEW eligibility case to the relevant visibility tests
(including the later cases referenced in the comment) by reusing the existing
test flow with a bounty fixture whose status is UNDER_REVIEW and asserting the
same eligible-user behavior. Keep the coverage aligned with the requirement by
updating the tests around MOCK_BOUNTY and the button visibility assertions so
both IN_PROGRESS and UNDER_REVIEW are verified.
- Around line 165-175: The mocked /api/disputes route in raise-dispute.spec.ts
is too permissive because it fulfills any request without validating the request
contract. Tighten the page.route handler by checking the request method and
asserting the JSON payload contains the expected dispute-submit fields
(campaignId, reason, description) before fulfilling, using the existing
disputeHandler branch and the default mock response path to keep the integration
test meaningful.
- Around line 177-186: The auth cookie setup in the raise-dispute e2e flow is
still hardcoded to localhost, which breaks when the test host comes from
BASE_URL. Update the cookie seeding in the relevant Playwright setup to use the
configured base URL/host instead of a fixed domain so it works for 127.0.0.1,
custom hostnames, and remote environments. Keep the change localized to the
cookie creation logic in the raise-dispute spec and ensure it derives the domain
from the same BASE_URL used by playwright.config.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c6d4e8c-9ec0-42bc-b8ae-6b9f79531c10
📒 Files selected for processing (2)
e2e/raise-dispute.spec.tsplaywright.config.ts
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
CI is fully green (build, lint, e2e all pass), tests cover every #280
acceptance criterion plus two bonuses, and the validation/toast strings
match the actual implementation in raise-dispute-dialog.tsx. Two things
before merge: please drop the playwright.config.ts changes (BASE_URL env
var support is out of scope for #280 — open a separate PR if it's needed
for your local workflow), and please reuse setupMocks from bounty-
application.spec.ts as #280 asked instead of redefining a fresh one with
a different signature inside this spec — export the existing function and
import it here, or extract a shared e2e/helpers/mocks.ts (PR #304 is also
extracting mocks, ideal would be to coordinate on one shared module).
Once those land, ready to merge.
|
Hey @DevSolex , |
On it. |
…est contract assertion, revert config
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
e2e/bounty-application.spec.ts (1)
130-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDon't expose reusable helpers from a
*.spec.tsfile.Any later spec that imports
setupMockswill also execute this module's top-leveltest.describe(...), registering the bounty suite as an import side effect. KeepsetupMocksprivate here, or move it intoe2e/helpers/before reusing it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/bounty-application.spec.ts` at line 130, The reusable helper setupMocks is being exported from a spec file, which can cause importing test modules to execute bounty suite side effects through the top-level test.describe. Make setupMocks private within the bounty-application.spec.ts module or move it into a shared e2e/helpers utility before reusing it, and keep the spec file focused only on registering its own tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/helpers/mocks.ts`:
- Around line 105-111: LEADERBOARD_STUBS no longer matches the GraphQL response
shapes that the detail-page tests already consume. Update the shared stub values
in mocks.ts so TopContributors, Leaderboard, GetLeaderboardUser, and
LeaderboardUser return the expected field structure used by the specs (for
example topContributors, leaderboard, and userLeaderboard), and keep the shared
helper aligned with the existing consumers in raise-dispute.spec.ts and
bounty-application.spec.ts.
In `@e2e/raise-dispute.spec.ts`:
- Around line 286-287: The spec has a duplicate capturedRequests declaration
that will break parsing/type-checking before Playwright runs. In
raise-dispute.spec.ts, remove the extra const capturedRequests definition and
keep only one declaration in the relevant test/setup scope, making sure any
later references in the same spec still use that single capturedRequests
variable.
---
Nitpick comments:
In `@e2e/bounty-application.spec.ts`:
- Line 130: The reusable helper setupMocks is being exported from a spec file,
which can cause importing test modules to execute bounty suite side effects
through the top-level test.describe. Make setupMocks private within the
bounty-application.spec.ts module or move it into a shared e2e/helpers utility
before reusing it, and keep the spec file focused only on registering its own
tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89c3df76-b69e-4868-8400-2ebce560958f
📒 Files selected for processing (3)
e2e/bounty-application.spec.tse2e/helpers/mocks.tse2e/raise-dispute.spec.ts
| const capturedRequests: { method: string; body: unknown }[] = []; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate capturedRequests declaration.
This block declares the same const twice, so the spec will fail to parse/type-check before Playwright can run it.
Minimal fix
- const capturedRequests: { method: string; body: unknown }[] = [];
const capturedRequests: { method: string; body: unknown }[] = [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const capturedRequests: { method: string; body: unknown }[] = []; | |
| const capturedRequests: { method: string; body: unknown }[] = []; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/raise-dispute.spec.ts` around lines 286 - 287, The spec has a duplicate
capturedRequests declaration that will break parsing/type-checking before
Playwright runs. In raise-dispute.spec.ts, remove the extra const
capturedRequests definition and keep only one declaration in the relevant
test/setup scope, making sure any later references in the same spec still use
that single capturedRequests variable.
Use makeSession() and LEADERBOARD_STUBS from e2e/helpers/mocks.ts so the spec matches the pattern raise-dispute.spec already follows. Also type makeSession's return as a MockSession interface so callers can access .user fields.
Reset branch onto current main so this PR no longer deletes the work from PRs boundlessfi#298, boundlessfi#311, and boundlessfi#313. Only changes left: - New hooks/__tests__/use-bounty-application.test.tsx with 12 cases covering happy and rollback paths for useApplyToBounty, useSelectApplicant, useApproveApplicationSubmission, useRequestRevisions, useApplyForSlot, useReleasePayment, useRemoveContributor, useDeclineApplicant, and useRaiseDispute. - Validation in useApplyToBounty that throws ApplicationError when applicantAddress is missing. Production guard, not test-only. Verified pnpm lint, pnpm tsc --noEmit, pnpm build, and the new test suite (12/12 passing) all clean.
2 participants
Closes #280
What
Adds
e2e/raise-dispute.spec.tscovering the full Raise Dispute flow as specified in #280.Tests (7 cases)
IN_PROGRESSbounty sees an active Raise Dispute buttonIN_PROGRESSbounty sees an active Raise Dispute buttonAlertDialogwith a reasonSelectand descriptionTextarea/dispute/:idConventions followed
MOCK_SESSION/setupMockspattern frome2e/bounty-application.spec.tsPOST /api/disputesmocked viapage.route('**/api/disputes', ...)await expect(...)Verified
Summary by CodeRabbit